Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Only create layer env directories if env vars are being written #385

Merged
merged 1 commit into from
Mar 29, 2022

Conversation

edmorley
Copy link
Member

Prevents empty env, env.build and env.launch directories from being created within each layer's directory, if no environment variables are set within them.

Fixes #384.
GUS-W-10905886.

Prevents empty `env`, `env.build` and `env.launch` directories from being
created within each layer's directory, if no environment variables are set within
them.

Fixes #384.
GUS-W-10905886.
@edmorley edmorley self-assigned this Mar 27, 2022
@edmorley
Copy link
Member Author

There wasn't an obvious existing test to adapt or duplicate - I'll let you decide whether we need a new test for this or not :-)

@edmorley edmorley requested a review from Malax March 27, 2022 18:02
@edmorley edmorley added the enhancement New feature or request label Mar 27, 2022
Copy link
Member

@Malax Malax left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the fix! 👍🏻

I was thinking about recommending to move the fs::create_dir_all into the for to avoid the extra check for emptiness, but in the end it does not really matter and might even have slightly worse runtime characteristics.

If anything, the comment threw me off at first since it (for me at least) comments the whole block, not only the if. But we can keep it there. :)

@edmorley edmorley merged commit 9144aba into main Mar 29, 2022
@edmorley edmorley deleted the edmorley/prevent-empty-env-dirs branch March 29, 2022 08:03
@edmorley
Copy link
Member Author

edmorley commented Mar 29, 2022

and might even have slightly worse runtime characteristics

I would think it would be definitely worse (albeit still not enough to be measurable) - one in-memory BTreeMap empty check vs multiple duplicate disk stats to confirm the directory already exists :-) (no doubt mitigated by the OS's disk cache, but still)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request libcnb
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Layer env directories are always created even if no layer env vars are set
2 participants